Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ingest Manager] Better handling of package installation problems #66541

Merged
merged 6 commits into from
May 18, 2020

Conversation

skh
Copy link
Contributor

@skh skh commented May 14, 2020

Summary

Fixes #65880

  • Improves error handling in the handler for the /api/ingest_manager/setup route for
    • missing package registry
    • errors during the installation of pre-built index templates
  • Fixes installation of pre-built index templates, caused by us not waiting for component templates to be installed before attempting to install dependent index templates.

The latter was most likely caused by changing behavior of elasticsearch.

Reviewers: please also comment if you agree that this should be backported to the 7.8 branch as a bugfix or not.

How to test this

Missing package registry

  • in kibana.dev.yml, or on the command line, specify a registry with xpack.ingestManager.epm.registryUrl: 'http://localhost:8080' but don't run it
  • login in to Kibana and watch the network tab in the browser dev console
  • the call to /api/ingest_manager/setup should return a 502 http response
  • Kibana outside Ingest Manager should be usable
  • If you navigate to Ingest Manager, the error box should contain an informative error message
  • the Kibana log should contain the same error message, and no stack trace

Missing component template

This fix contains two parts: how the error was handled, and why the error occurred in the first place.

  • To test the improved error handling, trigger the original error by commenting out this line: https://github.com/elastic/kibana/pull/66541/files#diff-51e0df792f23fb0552e1902bf96453e4R24

  • restart your local elasticsearch, or make sure the index templates and component templates from the base package are not installed

  • login to Kibana

  • the call to /api/ingest_manager/setup should return with a 400 http response containing the elasticsearch error message complaining about missing component templates

  • To test that the installation of pre-built index templates works, revert the change for the previous test

  • restart your local elasticsearch, or make sure the index templates and component templates from the base package are not installed

  • login to Kibana

  • the call to /api/ingest_manager/setup should return with a 200 http response

@skh skh self-assigned this May 14, 2020
@skh skh added Ingest Management:alpha1 Group issues for ingest management alpha1 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.8.0 v7.9.0 v8.0.0 labels May 14, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@skh skh marked this pull request as ready for review May 18, 2020 12:36
@skh skh requested a review from a team May 18, 2020 12:36
@@ -20,8 +21,8 @@ export const installTemplates = async (
// install any pre-built index template assets,
// atm, this is only the base package's global index templates
// Install component templates first, as they are used by the index templates
installPreBuiltComponentTemplates(pkgName, pkgVersion, callCluster);
installPreBuiltTemplates(pkgName, pkgVersion, callCluster);
await installPreBuiltComponentTemplates(pkgName, pkgVersion, callCluster);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this two operation concurently inside a Promise.all

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second depends on the first being completed, so we can't do them concurrently.

try {
return await Promise.all(templateInstallPromises);
} catch (e) {
throw new Boom(`Error installing prebuilt index templates ${e.message}`, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to start to introduce our domain errors and avoid using Boom ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to start with that in changes that don't go into 7.8 to keep the changes smaller.

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally and works as expected.

});
}
logger.error(e.message);
logger.error(e.stack);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neptunian This gives us a stack trace from the handler that's actually pointing at the real location the error occurred. I only added this here for "unknown" errors.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@skh skh merged commit 84e06af into elastic:master May 18, 2020
@skh skh deleted the 64067-robust-package-installation-ctd branch May 18, 2020 17:18
skh added a commit to skh/kibana that referenced this pull request May 18, 2020
…astic#66541)

* Better handle non-available package registry

* Log errors in route handler only

* Await installation of prebuilt component templates

* Remove leftover import

* Remove useless use of await.
skh added a commit to skh/kibana that referenced this pull request May 18, 2020
…astic#66541)

* Better handle non-available package registry

* Log errors in route handler only

* Await installation of prebuilt component templates

* Remove leftover import

* Remove useless use of await.
skh added a commit that referenced this pull request May 19, 2020
…6541) (#66916)

* Better handle non-available package registry

* Log errors in route handler only

* Await installation of prebuilt component templates

* Remove leftover import

* Remove useless use of await.
skh added a commit that referenced this pull request May 19, 2020
…6541) (#66917)

* Better handle non-available package registry

* Log errors in route handler only

* Await installation of prebuilt component templates

* Remove leftover import

* Remove useless use of await.
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 19, 2020
* master: (24 commits)
  [APM] agent config 'profiling_inferred_spans_min_duration' default value is '0ms' but the min value in the field is '1ms' (elastic#66886)
  [Canvas] Fix flaky custom element functional tests (elastic#65908)
  Fix IE specific flexbox min-height issue (elastic#66555)
  [Discover] Unskip doc link functional test (elastic#66884)
  Index pattern management to Kibana platform (elastic#65026)
  Warning and link to support matrix for IE11 (elastic#66512)
  [Reporting] Consolidate Server Type Defs, move some out of Legacy (elastic#66144)
  [SIEM] [Maps] Fixes Network Map empty tooltip (elastic#66828)
  [Endpoint] Encode the index of the alert in the id response (elastic#66919)
  [services/testSubjects] reduce retry usage, add waitForEnabled (elastic#66538)
  [DOCS] Identifies cloud settings for APM (elastic#66935)
  [SIEM][CASE] Fix configuration's page user experience (elastic#66029)
  Resolver: Display node 75% view submenus (elastic#64121)
  [SIEM] Cases] Capture timeline click and open timeline in case view (elastic#66327)
  [APM] Lowercase agent names so icons work (elastic#66824)
  [dev/cli] add support for --no-cache (elastic#66837)
  [Ingest Manager] Better handling of package installation problems (elastic#66541)
  [ML] Enhances api docs for modules endpoints (elastic#66738)
  dont hide errors (elastic#66764)
  [RFC] Global search API (elastic#64284)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ingest Management:alpha1 Group issues for ingest management alpha1 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.8.0 v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EPM] await call to InstallPrebuiltComponentTemplates
5 participants